Remove collector container and merge it with planner #46
Conversation
machacekondra
force-pushed
the
remove_collector
branch
2 times, most recently
from
def0a6c
to
90b9e38
Compare
Since collector is simple golang app, we don't need to run it as standalone container. This PR changes the collector to be run from planner-agent. Signed-off-by: Ondra Machacek <[email protected]>
machacekondra
force-pushed
the
remove_collector
branch
from
90b9e38
to
3c73995
Compare
| @@ -46,7 +46,7 @@ storage: | |||
| contents: | |||
| inline: | | |||
| PasswordAuthentication yes | |||
| - path: /home/core/vol/config.yaml | |||
| - path: /home/core/.migration-planner/config.yaml | |||
There was a problem hiding this comment.
shouldn't be .migration-planner/config/config.yaml?
There was a problem hiding this comment.
It can, but won't help us actually, as we need to pass the path as parameter anyway, if we don't use default /etc/planner/config as config-dir . But will change as it makes more sense.
| @@ -92,6 +92,9 @@ func (a *Agent) Run(ctx context.Context) error { | |||
| } | |||
| healthChecker.Start(healthCheckCh) | |||
|
|
|||
| collector := NewCollector(a.log, a.config.DataDir) | |||
| go collector.collect() | |||
There was a problem hiding this comment.
the context needs to be passed into the collector to be stopped gracefully.
There was a problem hiding this comment.
Why do you think? Currently we keep running the agent, which monitor the changes of inventory.json. Maybe we should keep it running just with noop, because noone can update the inventory.json, but if we keep it running podman-auto-update can update image and restart the pod, so we actually do update the invetory.json.
There was a problem hiding this comment.
I was speaking about stopping the goroutine. If you don't check the context it will leak at the end. Also, we must wait for the collector to close the inventory.json properly before exiting the agent.
| logger.Println("Wait for credentials") | ||
| waitForFile(credsFile) | ||
| c.log.Infof("Wait for credentials") | ||
| waitForFile(credentialsFilePath) |
There was a problem hiding this comment.
IMO, now that we've integrated the collector into the agent, I think it's better to pass the credentials as struct to the collector and start it only when we get the credentials.
wdyt?
There was a problem hiding this comment.
If we keep the credentials.json file we can restart the service (for example, if there is new image) and fetch new inventory. If we just send over credentials struct, we will loose it.
Since collector is simple golang app, we don't need to run it as standalone container. This PR changes the collector to be run from planner-agent.